Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove uses of DECODE_RETURN_KIND part of GCInfo #110799

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Dec 18, 2024

A follow-up to the GC info changes in 9.0.

Since the call sites now have complete liveness info, the part about return value kind[s] recorded in GC info is redundant.
Evidently, with CET enabled, hijacking already has no need for that.

This change removes all the uses of DECODE_RETURN_KIND in hijacking scenarios.

NOTE: The encoder still populates this part of GC info for backward-compat reasons. Decoder skips it.
Once we move forward the minimum R2R version, we can remove the return kind bits from the GC info format. This will save a few bits of GC info for each method and will allow more methods to use "slim" format.

X86: stays on the old plan.

Copy link
Contributor

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

// return values.
virtual unsigned GetFrameAttribs() {
LIMITED_METHOD_DAC_CONTRACT;
return FRAME_ATTR_RESUMABLE; // Treat the next frame as the top frame.
Copy link
Member Author

@VSadov VSadov Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key change.
On non-x86 we can treat a thread self-interrupted synchronously by a hijack the same as a thread interrupted asynchronously by a signal. The difference is the size of the captured context and who captured it.

In theory the hijack could capture the entire context as the OS interrupt mechanisms do, but that would be fairly redundant, so hijack captures only what we could possibly need.

There is a small catch here. If in the future we need to report more registers (say some ABI is extended to return up to 3 GC pointers), we would need to adjust OnHijackTripThread and HijackFrame::UpdateRegDisplay accordingly. If hijack captured the entire context as OS interrupts do, we would not have to adjust. Things would "just work".

I thought about this and, considering that a good portion of context cannot contain GC pointers at call site, not capturing the entire context seems the right approach.

@@ -117,7 +112,7 @@ NESTED_ENTRY RhpGcProbeHijack, _TEXT, NoHandler
ret

LOCAL_LABEL(WaitForGC):
or ecx, DEFAULT_FRAME_SAVE_FLAGS + PTFF_SAVE_RAX + PTFF_SAVE_RDX
mov ecx, DEFAULT_FRAME_SAVE_FLAGS + PTFF_SAVE_RAX + PTFF_SAVE_RDX + PTFF_THREAD_HIJACK
Copy link
Member Author

@VSadov VSadov Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PTFF_THREAD_HIJACK is the NativeAOT couterpart to the HijackFrame.
This is basically a way to mark hijacked frames as leaf/active/top frames.
(as opposed to regular transition frames, which are not leaf frames)

@VSadov VSadov requested a review from jkotas December 19, 2024 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant